-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not serialize optimize-queries option in MapConfig #16524
Do not serialize optimize-queries option in MapConfig #16524
Conversation
This setting is a derived value and when serializing there is no need to set it in the generated XML
…and setCacheDeserializedValues is set
run-lab-run |
@@ -401,6 +401,16 @@ public void givenSetOptimizeQueryIsTrue_whenSetCacheDeserializedValuesToINDEX_ON | |||
mapConfig.setCacheDeserializedValues(CacheDeserializedValues.INDEX_ONLY); | |||
} | |||
|
|||
@Test | |||
public void givenSetOptimizeQueryIsFalse_whenSetCacheDeserializedValuesToNEVER_thenNoException() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not that important but this is just one of possible 4 allowed combinations. I don't think we cover setOptimizeQueries(false)
followed by setCacheDeserializedValues(CacheDeserializedValues.INDEX_ONLY)
, and setCacheDeserializedValues(CacheDeserializedValues.NEVER/INDEX_ONLY)
followed by setOptimizeQueries(false)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the expected behaviour when INDEX_ONLY is set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually had another bug which wasn't covered. I've updated the tests and cleaned up the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually don't know what the expected behaviour is with INDEX_ONLY
, I simply looked at the current impl. INDEX_ONLY
is the default and when setOptimizeQueries(false)
is called, this default value does not change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so nothing should happen when it's set to false and it's set to INDEX_ONLY?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I actually don't know - maybe @jerrinot will be able to help?
371fd7b
to
9778cb7
Compare
I've changed it now that when optimizeQueries = |
2246ba3
to
9053b94
Compare
run-lab-run |
This setting is a derived value and when serializing there is no need to set it in the generated XML. Also fix the wrong conflict detection when both options are set.